Skip to content

Fix ref without mut parameter check on methods and associated functions#7633

Open
Dnreikronos wants to merge 4 commits into
FuelLabs:masterfrom
Dnreikronos:Dnreikronos/fix-ref-without-mut-check
Open

Fix ref without mut parameter check on methods and associated functions#7633
Dnreikronos wants to merge 4 commits into
FuelLabs:masterfrom
Dnreikronos:Dnreikronos/fix-ref-without-mut-check

Conversation

@Dnreikronos
Copy link
Copy Markdown
Contributor

@Dnreikronos Dnreikronos commented May 25, 2026 •
edited
Loading

Description

Fixes #7623.

ref without mut on a parameter was checked inconsistently. On a free function it raised a misleading call-site error ("Cannot pass immutable argument to mutable parameter"), and on methods and associated functions it slipped through with no check at all. The cause is VariableMutability::new_from_ref_mut(is_reference=true, is_mutable=false) returning RefMutable, so an immutable reference got treated as mutable and tripped the call-site mutability check.

The fix adds a RefImmutable variant so ref and ref mut are no longer conflated, and narrows the call-site check to fire only for ref mut params. A ref parameter now accepts immutable arguments everywhere while staying immutable inside the function body. ref mut behavior is unchanged.

Case Before After
ref param, immutable arg (free fn) spurious error allowed
ref param, immutable arg (method / associated fn) no check allowed
assigning to a ref param in the body allowed rejected
ref mut param, immutable arg error error

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (Forc, Sway book, etc.).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a) parsing test in core/trybuild-tests if needed.
  • I have added the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@Dnreikronos
Add RefImmutable variant to fix ref without mut parameter handling
a8a5620 
ref without mut incorrectly mapped to RefMutable in new_from_ref_mut,
causing ref-only parameters to be treated as mutable (wrong call-site
error on free fns, silently accepted on methods). Add RefImmutable
variant so ref-only parameters are correctly immutable both at call
sites and inside function bodies.

Closes FuelLabs#7623
@Dnreikronos
Add tests for ref without mut parameter behavior
280bc77 
should_pass: ref without mut on free fns, methods, and associated fns
accepts immutable arguments and returns correct values.

should_fail: ref without mut params are immutable inside body (cannot
assign), and ref mut params still reject immutable arguments.
@Dnreikronos Dnreikronos requested a review from a team as a code owner May 25, 2026 23:32
@cursor
Copy link
Copy Markdown

cursor Bot commented May 25, 2026 •
edited
Loading

PR Summary

Medium Risk
Changes core parameter mutability semantics in the compiler; behavior shifts for existing ref parameters but is localized to reference/mutability checking with new tests.

Overview
Fixes inconsistent handling of ref parameters (without mut) by distinguishing them from ref mut in the type checker.

Adds VariableMutability::RefImmutable and updates new_from_ref_mut so reference-only parameters are no longer classified as RefMutable. That wrong classification caused free functions to reject valid calls with “immutable argument to mutable parameter,” while methods/associated functions skipped the check entirely.

The call-site mutability rule in unify_arguments_and_parameters now fires only when the parameter is ref mut (is_reference && is_mutable), not for plain ref. Immutable ref params still block assignment inside the function body via RefImmutable not being mutable. Declaration pretty-printing includes the new variant.

E2E tests cover passing immutable args to ref on free functions, methods, and associated fns, plus failures for assigning to ref params and for ref mut with immutable args.

Reviewed by Cursor Bugbot for commit 7f350f4. Bugbot is set up for automated code reviews on this repo. Configure here.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 4, 2026

Merging this PR will not alter performance

✅ 25 untouched benchmarks

Comparing Dnreikronos:Dnreikronos/fix-ref-without-mut-check (7f350f4) with master (ac933ff)

Open in CodSpeed

@Dnreikronos
Copy link
Copy Markdown
Contributor Author

Dnreikronos commented Jun 5, 2026

bugbot run

cursor[bot]
cursor Bot reviewed Jun 5, 2026
View reviewed changes
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7f350f4. Configure here.

@Dnreikronos
Copy link
Copy Markdown
Contributor Author

Dnreikronos commented Jun 5, 2026

@ironcev ready for review. Adds VariableMutability::RefImmutable so a plain ref param isn't classified as RefMutable, and gates the call-site check on ref mut only. The open question is that new variant: want to be sure it's the right place to split ref from ref mut.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

cla:signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ref without mut check not done on methods and associated functions

1 participant

@Dnreikronos